Optional Recursive Mutex in Asio Integration#1846
Optional Recursive Mutex in Asio Integration#1846RobertLeahy wants to merge 2 commits intoNVIDIA:mainfrom
Conversation
Reduces the size of a "frame" by removing the std::unique_lock member variable and making the "frame" itself the lock guard. This is the lock management method shown when presenting asioexec::completion_token in the CppCon 2025 talk "std::execution in Asio Codebases: Adopting Senders Without a Rewrite."
For general purpose (i.e. potentially multithreaded) use the asynchronous operations which result when passing the asioexec:: completion_token and ::use_sender completion tokens must use a recursive mutex internally. However if the user knows that no multithreaded use will occur this recursive mutex is pure overhead. Provided the asioexec::thread_unsafe_completion_token and _use_sender completion tokens which do not make use of a recursive mutex for the aforementioned use case.
| Receiver r_; | ||
| asio_impl::cancellation_signal signal_; | ||
| std::recursive_mutex m_; | ||
| [[no_unique_address]] |
There was a problem hiding this comment.
in operation states, this should be STDEXEC_IMMOVABLE_NO_UNIQUE_ADDRESS
| const auto prev = self.frames_->prev_; | ||
| self.frames_->prev_ = nullptr; | ||
| self.frames_ = prev; | ||
| self.m_.unlock(); | ||
| if (!prev) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
why not use std::exchange?
| const auto prev = self.frames_->prev_; | |
| self.frames_->prev_ = nullptr; | |
| self.frames_ = prev; | |
| self.m_.unlock(); | |
| if (!prev) { | |
| break; | |
| } | |
| self.frames_ = std::exchange(self.frames_->prev_, nullptr); | |
| self.m_.unlock(); | |
| if (!self.frames_) { | |
| break; | |
| } |
| @@ -326,7 +343,7 @@ namespace asioexec { | |||
| [&](auto&&... args) { | |||
| std::invoke( | |||
There was a problem hiding this comment.
STDEXEC::__invoke compiles about ~2x faster than libstdc++'s std::invoke.
There was a problem hiding this comment.
Would be a drive by, can put in a separate commit.
| : self_([&]() noexcept { | ||
| self.m_.lock(); | ||
| return &self; | ||
| }()) |
There was a problem hiding this comment.
lambdas are expensive at compile time. this could simply be:
| : self_([&]() noexcept { | |
| self.m_.lock(); | |
| return &self; | |
| }()) | |
| : self_((self.m_.lock(), &self)) |
or if you find that distasteful, you could use a delegating constructor:
explicit frame_(operation_state_base& self) noexcept
: frame_(std::unique_lock(self.m_), self) {
}
private:
explicit frame_(std::unique_lock<Mutex> lk, operation_state_base& self) noexcept
: self_(&self) {
lk.release();
}| }; | ||
|
|
||
| template <typename Mutex> | ||
| struct t { |
There was a problem hiding this comment.
please give this a more descriptive name
There was a problem hiding this comment.
detail::completion_token::completion_token felt needless, but it can be named whatever.
|
|
||
| template <typename Signatures, typename Receiver, typename Allocator> | ||
| template <typename Mutex, typename Signatures, typename Receiver, typename Allocator> | ||
| requires requires(const Receiver& r) { ::STDEXEC::get_allocator(::STDEXEC::get_env(r)); } |
There was a problem hiding this comment.
| requires requires(const Receiver& r) { ::STDEXEC::get_allocator(::STDEXEC::get_env(r)); } | |
| requires STDEXEC::__callable<STDEXEC::get_allocator_t, STDEXEC::env_of_t<Receiver>> |
There was a problem hiding this comment.
This would be a drive by, I can fix it in a separate commit (maybe as part of this PR?).
| explicit sender(Sender) -> sender<Sender>; | ||
|
|
||
| template <typename Mutex> | ||
| struct t { |
|
/ok to test 72c179e |
|
@RobertLeahy did you see my comments? also, this PR needs a rebase. |
Yes, just haven't looped back to incorporating any of the feedback. |
2 participants
Please do not squash.
Resolves #1781.